Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Group possible backend share configurations under same environment #117

Merged
merged 8 commits into from
Aug 28, 2024
Merged

Group possible backend share configurations under same environment #117

merged 8 commits into from
Aug 28, 2024

Conversation

anoopcs9
Copy link
Collaborator

@anoopcs9 anoopcs9 commented Aug 9, 2024 •
edited
Loading

Key considerations:

  • cephfs and cephfs.vfs are combined together under cephfs.
    • cephfs.mgr.vfs becomes cephfs.mgr when shares are configured using ceph smb mgr module.
  • A new methods field to indicate how variations of each share section are to be configured within smb.conf.
    • Special cases:
      • kernel client approach is not yet supported by ceph mgr module. Accordingly we skip this variation.
      • gpfs scale variant configuration is always based on vfs. Thus we skip iterating over available methods.

fixes #101

@anoopcs9 anoopcs9 force-pushed the group-backend-shares branch 3 times, most recently from 5a9628f to b84556a Compare August 10, 2024 11:05
@anoopcs9 anoopcs9 mentioned this pull request Aug 10, 2024
Merged
@anoopcs9 anoopcs9 marked this pull request as ready for review August 11, 2024 05:27
@anoopcs9
Copy link
Collaborator Author

I have another branch(group-backend-shares2) which reduces the complexity of changes from test-info.yml.j2 by defining the strategies within global settings.yml.

@anoopcs9
Copy link
Collaborator Author

anoopcs9 commented Aug 14, 2024 •
edited
Loading

/retest centos-ci/gpfs.scale

@spuiuk
Copy link
Collaborator

spuiuk commented Aug 14, 2024

Notes: Shares created

cephfs:
share-cephfs-default-vfs
share-cephfs-default-klcient

cephmgr:
share-cephfs-mgr-vfs

spuiuk
spuiuk previously approved these changes Aug 14, 2024
View reviewed changes
Copy link
Collaborator

@spuiuk spuiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. ACK

anoopcs9 added a commit to anoopcs9/samba-centosci that referenced this pull request Aug 15, 2024
@anoopcs9
fs-integration: Remove already grouped duplicate variants
6d26707 
With [1] all possible share configurations are grouped after their
respective backend file system. So those duplicate entries can be
removed reducing the number of jobs in the matrix.

[1] samba-in-kubernetes/sit-environment#117
anoopcs9 added a commit to anoopcs9/samba-centosci that referenced this pull request Aug 15, 2024
@anoopcs9
fs-integration: Use the new name for mgr variant of cephfs
46a1763 
With [1] suffix 'vfs' is irrelevant in 'cephfs.mgr.vfs' as multiple
share configurations are included within mgr variant.

[1] samba-in-kubernetes/sit-environment#117
@anoopcs9 anoopcs9 mentioned this pull request Aug 15, 2024
Merged
xhernandez
xhernandez reviewed Aug 27, 2024
View reviewed changes
Copy link
Collaborator

@xhernandez xhernandez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the meaning of variant and method (or strategy) is a bit mixed. Why do we need variant if we already have another list that contains all the variants we want to use ?

playbooks/ansible/roles/sit.cephfs/templates/smb_share.conf.j2 Show resolved Hide resolved
playbooks/settings.yml Outdated Show resolved Hide resolved
anoopcs9
anoopcs9 commented Aug 27, 2024
View reviewed changes
Copy link
Collaborator Author

@anoopcs9 anoopcs9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the meaning of variant and method (or strategy) is a bit mixed. Why do we need variant if we already have another list that contains all the variants we want to use ?

I intent to change the meaning of variant to indicate how(or who) Samba and/or CTDB configurations are handled during cluster setup. With this we will have just two variants namely, default(independent setup style) and mgr(using ceph mgr module). Each of these variants can have different methods through which shares are configured for client access.

playbooks/ansible/roles/sit.cephfs/templates/smb_share.conf.j2 Show resolved Hide resolved
playbooks/settings.yml Outdated Show resolved Hide resolved
@xhernandez
Copy link
Collaborator

I intent to change the meaning of variant to indicate how(or who) Samba and/or CTDB configurations are handled during cluster setup. With this we will have just two variants namely, default(independent setup style) and mgr(using ceph mgr module). Each of these variants can have different methods through which shares are configured for client access.

This means that the variant only makes sense for the ceph backend. All other backends won't have variants, which is a bit weird.

I think that we should move the definition of backends inside settings.yml (see #119). Then we could easily extend the definition like this (just a possible way, not thought much about it):

backends:
  cephfs:
    provider: mgr
    settings:
        <optional mgr specific options>
    shares:
      kclient:
        <optional kclient specific options>
      vfs:
        <optional vfs specific options>

This way we could easily create more than one backend, with multiple shares, in a single configuration (additional work required to avoid using the same devices, but that's another problem).

In any case, this is not related to this patch. We can discuss this in #119.

@anoopcs9
config.yml: Define share configuration methods
516e2f6 
Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
@anoopcs9
settings-cephfs: Add a new 'methods' field
309de62 
Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
@anoopcs9
sit.cephfs: Prepare and create shares based on methods
a3ed636 
Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
@anoopcs9
samba.setup: Avoid running cephfs tasks based on variant
5eebf9b 
Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
@anoopcs9
settings-gpfs: Add a new 'methods' field
4c7d7c7 
Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
@anoopcs9
sit.gpfs: Prepare and create shares based on methods
736c86c 
Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
@anoopcs9
samba.setup: Avoid running gpfs tasks based on variant
f658e1e 
Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
@anoopcs9
sit-test-cases: Modify test-info.yml to define all shares
b5b3cf3 
Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
@anoopcs9
Copy link
Collaborator Author

I intent to change the meaning of variant to indicate how(or who) Samba and/or CTDB configurations are handled during cluster setup. With this we will have just two variants namely, default(independent setup style) and mgr(using ceph mgr module). Each of these variants can have different methods through which shares are configured for client access.

This means that the variant only makes sense for the ceph backend. All other backends won't have variants, which is a bit weird.

I have considered scale as a variant for GPFS.

I think that we should move the definition of backends inside settings.yml (see #119). Then we could easily extend the definition like this (just a possible way, not thought much about it):

backends:
  cephfs:
    provider: mgr
    settings:
        <optional mgr specific options>
    shares:
      kclient:
        <optional kclient specific options>
      vfs:
        <optional vfs specific options>

This way we could easily create more than one backend, with multiple shares, in a single configuration (additional work required to avoid using the same devices, but that's another problem).

In any case, this is not related to this patch. We can discuss this in #119.

Sure.

For now I've updated to include the suggestion to use only methods avoiding the strategy keyword.

anoopcs9 added a commit to anoopcs9/sit-test-cases that referenced this pull request Aug 28, 2024
@anoopcs9
smbtorture: Find flapping list based on share name
0c84fa7 
With samba-in-kubernetes/sit-environment#117,
'variant' no longer indicates the method through which a particular
share is configured for client access. Instead determine the need for
VFS specific flapping file using the share name itself.

Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
anoopcs9 added a commit to anoopcs9/sit-test-cases that referenced this pull request Aug 28, 2024
@anoopcs9
smbtorture: Find flapping list based on share name
7db80c4 
With samba-in-kubernetes/sit-environment#117,
'variant' no longer indicates the method through which a particular
share is configured for client access. Instead determine the need for
VFS specific flapping file using the share name itself.

Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
spuiuk
spuiuk approved these changes Aug 28, 2024
View reviewed changes
Copy link
Collaborator

@spuiuk spuiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@anoopcs9 anoopcs9 merged commit 8d260b5 into samba-in-kubernetes:main Aug 28, 2024
10 checks passed
@anoopcs9 anoopcs9 deleted the group-backend-shares branch August 28, 2024 09:00
anoopcs9 added a commit to samba-in-kubernetes/samba-centosci that referenced this pull request Aug 28, 2024
@anoopcs9
fs-integration: Remove already grouped duplicate variants
bbc40d7 
With [1] all possible share configurations are grouped after their
respective backend file system. So those duplicate entries can be
removed reducing the number of jobs in the matrix.

[1] samba-in-kubernetes/sit-environment#117
anoopcs9 added a commit to samba-in-kubernetes/samba-centosci that referenced this pull request Aug 28, 2024
@anoopcs9
fs-integration: Use the new name for mgr variant of cephfs
ef20a6e 
With [1] suffix 'vfs' is irrelevant in 'cephfs.mgr.vfs' as multiple
share configurations are included within mgr variant.

[1] samba-in-kubernetes/sit-environment#117
@anoopcs9 anoopcs9 mentioned this pull request Aug 28, 2024
Open
anoopcs9 added a commit to samba-in-kubernetes/samba-centosci that referenced this pull request Aug 28, 2024
@anoopcs9
gitlab-integration: Remove already grouped duplicate variants
34c8ebb 
With [1] all possible share configurations are grouped after their
respective backend file system. So those duplicate entries can be
removed reducing the number of jobs in the matrix.

[1] samba-in-kubernetes/sit-environment#117
anoopcs9 added a commit to samba-in-kubernetes/samba-centosci that referenced this pull request Aug 28, 2024
@anoopcs9
gitlab-integration: Use the new name for mgr variant of cephfs
1676fc0 
With [1] suffix 'vfs' is irrelevant in 'cephfs.mgr.vfs' as multiple
share configurations are included within mgr variant.

[1] samba-in-kubernetes/sit-environment#117
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xhernandez xhernandez xhernandez approved these changes

@spuiuk spuiuk spuiuk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Combine multiple variants within same environment
3 participants
@anoopcs9 @spuiuk @xhernandez